update Cacheruntime integration doc and add english version#5794
update Cacheruntime integration doc and add english version#5794fluid-e2e-bot[bot] merged 3 commits intofluid-cloudnative:masterfrom
Conversation
Signed-off-by: xliuqq <xlzq1992@gmail.com>
Signed-off-by: xliuqq <xlzq1992@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive CacheRuntime Integration Guide in both English and Chinese, detailing the steps for installation, topology planning, and template preparation for Fluid. The review feedback identifies several technical inaccuracies and typographical errors in the documentation, such as incorrect Kubernetes API versions ('appv1' instead of 'apps/v1'), inconsistent capitalization of 'DaemonSet', and spelling mistakes like 'intergration' and 'poxy'. Additionally, a suggestion was made to clarify an ambiguous reference to the master component configuration.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5794 +/- ##
=======================================
Coverage 58.46% 58.46%
=======================================
Files 473 473
Lines 32222 32222
=======================================
Hits 18839 18839
Misses 11836 11836
Partials 1547 1547 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
Pull request overview
Updates the CacheRuntime integration documentation and adds an English version to help developers integrate new CacheRuntime implementations.
Changes:
- Updated the Chinese CacheRuntime integration guide content (topology/template/config examples).
- Added an English CacheRuntime integration guide.
- Linked both guides from the Chinese and English documentation TOCs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| docs/zh/dev/generic_cache_runtime_integration.md | Refreshes CacheRuntime integration guidance and examples (CN). |
| docs/zh/TOC.md | Adds the CN guide entry to the Developer Guide TOC. |
| docs/en/dev/generic_cache_runtime_integration.md | Introduces the English CacheRuntime integration guide. |
| docs/en/TOC.md | Adds the EN guide entry to the Developer Guide TOC. |
Comments suppressed due to low confidence (6)
docs/zh/dev/generic_cache_runtime_integration.md:51
- In the topology examples,
workLoadTypeis not the actual CacheRuntimeClass field name (the spec usesworkloadType). Using a different spelling/casing here can mislead readers when they write YAML manifests.
docs/zh/dev/generic_cache_runtime_integration.md:50 - Typo in the table:
Wokershould beWorker(appears multiple times in this section).
docs/zh/dev/generic_cache_runtime_integration.md:76 Topology中comopentcontains a typo; should becomponentto avoid confusion for readers.
docs/zh/dev/generic_cache_runtime_integration.md:85- The
Dependencies -> EncryptOptiondescription is internally inconsistent: it says the feature is not supported in the current version, but also says to use the Dataset-defined keys for access. Please clarify the actual behavior (e.g., either mark as supported and describe how, or remove the guidance and consistently mark it unsupported).
docs/zh/dev/generic_cache_runtime_integration.md:59 - Kubernetes kind name should be
DaemonSet(capital S). The table currently usesDaemonset, which is inconsistent with the YAML example below and Kubernetes API conventions.
docs/zh/dev/generic_cache_runtime_integration.md:132 - The CacheRuntimeClass schema uses
template:(seeapi/v1alpha1/cacheruntimeclass_types.go/ generated CRD), but the example YAML usespodTemplateSpec:. Readers following this example will end up with an invalid manifest; please update the example to use the current API field name.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Options | Default options, will be overridden by user settings | | | ||
| | Template | PodTemplateSpec native field | | | ||
| | Service | Currently only supports Headless | | | ||
| | Dependencies | EncryptOption | Whether this component needs Fluid to mount the access keys defined in Dataset for accessing data sources [Not supported in current version], using the keys defined in Dataset for access. | |
There was a problem hiding this comment.
The Dependencies -> EncryptOption description is internally inconsistent: it says the feature is not supported in the current version, but also says to use the Dataset-defined keys for access. Please clarify the actual behavior and keep it consistent with the YAML examples below.
| | Dependencies | EncryptOption | Whether this component needs Fluid to mount the access keys defined in Dataset for accessing data sources [Not supported in current version], using the keys defined in Dataset for access. | | |
| | Dependencies | EncryptOption | Whether this component needs Fluid to mount the access keys defined in Dataset for accessing data sources. | |
| service: # Need to create Headless Service for master, only supported when workloadType is StatefulSet | ||
| headless: {} | ||
| dependencies: | ||
| encryptOption: {} # Current not support |
There was a problem hiding this comment.
Grammar in the YAML comment: Current not support should be Currently not supported (or similar).
| encryptOption: {} # Current not support | |
| encryptOption: {} # Currently not supported |
| dependencies: | ||
| encryptOption: {} # Need to provide encryptOption declared by user in dataset for client |
There was a problem hiding this comment.
This client example suggests dependencies.encryptOption is required, but earlier in the doc it’s described as not supported in the current version. Please make the example and the stated feature support consistent.
| dependencies: | |
| encryptOption: {} # Need to provide encryptOption declared by user in dataset for client | |
| dependencies: {} |
| # Installation | ||
|
|
||
| * Install Fluid version that supports CacheRuntime. | ||
|
|
||
|
|
||
| ```shell | ||
| helm repo add fluid https://fluid-cloudnative.github.io/charts | ||
|
|
||
| helm repo update | ||
|
|
||
| helm search repo fluid --devel | ||
|
|
||
| helm install fluid fluid/fluid --devel --version xxx -n fluid-system | ||
| ``` | ||
|
|
||
| # Integration | ||
|
|
||
| ## Step 1. Plan Cluster Topology |
There was a problem hiding this comment.
The document uses multiple top-level # headings (# Installation, # Integration) after the title. Other docs in this repo use a single # for the page title and ## for sections; please demote these headings to keep a consistent structure.
| | Master | | * workLoadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* UFS mount command<br> <br>* HeadlessService needs to be created<br> <br>* Authentication keys need to be mounted | | ||
| | Worker: Used for single worker role definition | | * workLoadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* HeadlessService needs to be created<br> <br>* Authentication keys do NOT need to be mounted<br> <br>* TieredStore needs to be configured | | ||
| | Client | Fuse | * Role: Posix client<br> <br>* workLoadType: apps/v1/DaemonSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* Authentication parameters do NOT need to be mounted<br> <br>* TieredStore is NOT supported | | ||
|
|
||
| * P2P Worker: JuiceFS | ||
|
|
||
|
|
||
| | Topology | Settings | | ||
| | --- | --- | | ||
| | Worker: Used for single worker role definition | * workLoadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* HeadlessService<br> <br>* Authentication parameters need to be mounted<br> <br>* TieredStore is supported | | ||
| | Client | * Role: Fuse client<br> <br>* workLoadType: apps/v1/DaemonSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* Service is NOT required<br> <br>* Authentication parameters need to be mounted<br> <br>* TieredStore is supported | |
There was a problem hiding this comment.
In the topology examples, workLoadType is not the actual CacheRuntimeClass field name (the spec uses workloadType). Using a different spelling/casing here can mislead readers when they write YAML manifests.
| | Master | | * workLoadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* UFS mount command<br> <br>* HeadlessService needs to be created<br> <br>* Authentication keys need to be mounted | | |
| | Worker: Used for single worker role definition | | * workLoadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* HeadlessService needs to be created<br> <br>* Authentication keys do NOT need to be mounted<br> <br>* TieredStore needs to be configured | | |
| | Client | Fuse | * Role: Posix client<br> <br>* workLoadType: apps/v1/DaemonSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* Authentication parameters do NOT need to be mounted<br> <br>* TieredStore is NOT supported | | |
| * P2P Worker: JuiceFS | |
| | Topology | Settings | | |
| | --- | --- | | |
| | Worker: Used for single worker role definition | * workLoadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* HeadlessService<br> <br>* Authentication parameters need to be mounted<br> <br>* TieredStore is supported | | |
| | Client | * Role: Fuse client<br> <br>* workLoadType: apps/v1/DaemonSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* Service is NOT required<br> <br>* Authentication parameters need to be mounted<br> <br>* TieredStore is supported | | |
| | Master | | * workloadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* UFS mount command<br> <br>* HeadlessService needs to be created<br> <br>* Authentication keys need to be mounted | | |
| | Worker: Used for single worker role definition | | * workloadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* HeadlessService needs to be created<br> <br>* Authentication keys do NOT need to be mounted<br> <br>* TieredStore needs to be configured | | |
| | Client | Fuse | * Role: Posix client<br> <br>* workloadType: apps/v1/DaemonSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* Authentication parameters do NOT need to be mounted<br> <br>* TieredStore is NOT supported | | |
| * P2P Worker: JuiceFS | |
| | Topology | Settings | | |
| | --- | --- | | |
| | Worker: Used for single worker role definition | * workloadType: apps/v1/StatefulSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* HeadlessService<br> <br>* Authentication parameters need to be mounted<br> <br>* TieredStore is supported | | |
| | Client | * Role: Fuse client<br> <br>* workloadType: apps/v1/DaemonSet<br> <br>* Image configuration<br> <br>* Startup command<br> <br>* Service is NOT required<br> <br>* Authentication parameters need to be mounted<br> <br>* TieredStore is supported | |
| encryptOption: {} # Current not support | ||
| podTemplateSpec: | ||
| spec: | ||
| restartPolicy: Always |
There was a problem hiding this comment.
The CacheRuntimeClass schema uses template: (see the generated CRD), but the example YAML uses podTemplateSpec:. Readers following this example will end up with an invalid manifest; please update the example to use the current API field name.
|
Thanks for the doc update. A few notes: Good changes:
One remaining typo in the Chinese version: Otherwise this looks good. Minor doc issue, not blocking. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ⅰ. Describe what this PR does
update Cacheruntime integration doc and add english version.
Ⅱ. Does this pull request fix one issue?
fixes #XXXX
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews